Initial precompiled shaders implementation #7834

SupaMaggie70Incorporated · 2025-06-20T04:15:06Z

Connections
Works towards #3103
Depends on #7831

Also, as this is the start of a big change, I'd like to ping @cwfitzgerald to at least make sure I haven't gone off in the wrong direction.

Description
This adds a CreateShaderModuleDescriptorPassthrough::Generic enum variant which contains code for multiple types of shader source,allows creating passthrough shaders without writing backend specific code (if on metal pass MSL, etc). This variant also includes an optional reflection thing. For now, I don't know exactly what reflection info is needed, but if possible, we should make sure this can live in wgpu-types or wgpu-core to avoid dependency on naga. It seems the best model for this is the wgpu_core::validation::Interface, we might just have to replace some of the naga types.

Using this requires enabling the EXPERIMENTAL_PRECOMPILED_SHADERS feature. This feature is only supported on DX12, Vulkan, and Metal. Logic on these backends is otherwise identical to the respective specific passthrough methods.

Nothing is currently done with the reflection info.

An overview of my approach can be found in this comment. If this process takes longer than expected we can make a tracking issue. For now I will be posting updates by editing that comment and referencing it in PRs.

Testing
No testing yet. However, the code seems somewhat small and robust.

Squash pls

Checklist

Run cargo fmt.
Run taplo format.
Run cargo clippy --tests. If applicable, add:
- --target wasm32-unknown-unknown
Run cargo xtask test to run tests.
If this contains user-facing changes, add a CHANGELOG.md entry.

…orporated/wgpu into dxil-passthrough

cwfitzgerald · 2025-06-25T19:57:20Z

Hmm, I think in general I like the ideas put forward in your plan you wrote up, that makes sense to me. As for the code changes themselves, I don't think that we need another variant internally - this feels redundant when all the other variants exist. Maybe what would need to be added is an optional reflection info to each of the variants?

SupaMaggie70Incorporated · 2025-06-25T21:28:57Z

@cwfitzgerald The purpose of this variant is so that the user can pass a single shader without considering backend-specific code. This way we can also later on add a macro that creates this struct, filling fields for all the backends that are supported, and it doesn't need to be passed a Backend parameter. I can change the approach if you want, but it might make everything a little less clean from the user's point of view.

cwfitzgerald · 2025-06-25T21:51:08Z

Yeah, I agree that it makes sense in wgpu's api but wgpu-core and wgpu-hal should be able to use the original ones?

SupaMaggie70Incorporated · 2025-06-25T21:58:06Z

Yeah, I agree that it makes sense in wgpu's api but wgpu-core and wgpu-hal should be able to use the original ones?

True, didn't think about that to be honest. I'll rewrite that part of this PR

cwfitzgerald

I added some comments on the code, I want to do some docs adjustments before this lands, but github won't let me do it inline, so I'll push a commit when I have a minute

wgpu-hal/src/dx12/device.rs

wgpu-hal/src/metal/device.rs

wgpu-hal/src/vulkan/device.rs

cwfitzgerald · 2025-07-10T23:09:21Z

wgpu-types/src/lib.rs

+        if let Some(spirv) = &self.spirv {
+            bytemuck::cast_slice(spirv)
+        } else if let Some(msl) = &self.msl {
+            msl.as_bytes()
+        } else if let Some(dxil) = &self.dxil {
+            dxil
+        } else {
+            panic!("No binary data provided to `ShaderModuleDescriptorGeneric`")


I don't think we can have a single data file anymore, we need to have a single file for each type. Additionally we need to output the hlsl etc. With tracing we need to be able to replicate all the descriptors.

@cwfitzgerald I wonder if for tracing we should only use the source that is actually used. And yeah good catch, this was just cobbled together so it would still build and have "some" level of tracing

I think the intent was that traces should be able to work cross api, but I don't know if that is currently true. The trace infrastructure is broken and pretty neglected.

I've attempted to address this. Let me know if you think my approach is workable. I haven't done any testing or thought too hard about it, but as you mentioned tracing is in a very sorry state right now.

SupaMaggie70Incorporated · 2025-07-16T01:28:36Z

I added some comments on the code, I want to do some docs adjustments before this lands, but github won't let me do it inline, so I'll push a commit when I have a minute

All your comments should be addressed :)

jimblandy · 2025-08-06T15:17:57Z

@cwfitzgerald When you're back - this has been waiting for a while.

DJMcNab

I'm really looking forward to this! Just some things which popped up!

player/src/lib.rs

wgpu-types/src/features.rs

DJMcNab · 2025-08-06T15:40:58Z

wgpu-types/src/features.rs

@@ -1233,15 +1210,18 @@ bitflags_array! {
        /// [BlasTriangleGeometrySizeDescriptor::vertex_format]: super::BlasTriangleGeometrySizeDescriptor
        const EXTENDED_ACCELERATION_STRUCTURE_VERTEX_FORMATS = 1 << 50;

-        /// Enables creating shader modules from DirectX HLSL or DXIL shaders (unsafe)
+        /// Enables creating shaders from passthrough with reflection info (unsafe)


Can we link to the relevant API (even though it will need to be hardcoded to docs.rs). E.g. how we linked to include_spirv_raw.

Is there a good reason why we should do that instead of e.g. just giving the function name? Linking directly to docs.rs seems like it would come with some potential issues, like linking to more recent or non-existent docs as the API changes (or before it is part of a release)

It's not super important. Again, I was mostly seeing that it was incongruent with the old API, which did link to include_spirv_raw - though for some reason, calling it make_spirv_raw. See also Self::PUSH_CONSTANTS, as an example.

That is, these concerns have already been litigated, I believe.

wgpu-types/src/lib.rs

DJMcNab · 2025-08-06T15:43:46Z

wgpu-types/src/lib.rs

+    /// Optional reflection information
+    pub reflection: Option<ShaderModuleReflection>,


We should better document when it is valid to leave this out/what is it used for/how do you get it?

Also missing full stop at end of doc comment.

Reflection information is always optional. I left this as a field so we could work from it later on. Right now the struct is actually empty. There isn't a way to actually construct it yet. I can remove it if you think thats best for now, just wanted to get out a rough sketch of the API.

SupaMaggie70Incorporated · 2025-08-06T16:00:19Z

@DJMcNab Thanks for the review! Upon further inspection it seems my PR was not super well thought through or checked, so I'll have another look over everything myself in addition to addressing your comments.

SupaMaggie70Incorporated · 2025-08-09T05:28:46Z

@DJMcNab I've just given this PR a final look and it looks good. Some of your comments I left open as I wasn't sure how to handle them. Let me know what you think, you get the final say of course!

Also pinging @cwfitzgerald because this hopefully can be merged soon

Another note: the only backend not currently supported is OpenGL. Once we get that done, maybe we should consider removing this as a feature? I'm not sure how that would interact with e.g. custom backends so I'll wait until later to inquire about that.

DJMcNab · 2025-08-11T08:06:55Z

I'm not planning to review this again - it needs to be reviewed by someone with write access, and I was providing a "first pass" review to help speed this along, but I can't give a fully "in-context" review.

Thanks for actioning the review feedback so well! I've found it easy to submit PRs which don't hold up when given a second reading myself as well - that's what review is for, after all!

SupaMaggie70Incorporated added 15 commits June 19, 2025 03:38

Unsure but these were prior changes

21e3bfd

Merge branch 'dxil-passthrough' of https://github.com/supamaggie70inc…

d9e089d

…orporated/wgpu into dxil-passthrough

Fixed feature thing

818b1fe

Mostly finished dxil passthrough

873a0ee

Attempted to add HLSL passthrough

da84fed

Tried to fix compiles

5680c9e

Tried to fix more clippy lints

05b8cda

Added initial feature, going back to fix issues with DXIL rn

4c28f2a

Tried to fix some potential issues

d050a20

Merge branch 'dxil-passthrough' into precompiled-shaders

5c1c8c8

Initial work

3ca8ce4

Tried to fix metal compiles

f7ffe36

Fixed warning

5006d54

Added my ideas for changes

d69e277

Tried to expose precompiled shaders to metal

3f9b7b8

SupaMaggie70Incorporated marked this pull request as ready for review June 21, 2025 02:45

SupaMaggie70Incorporated requested a review from a team as a code owner June 21, 2025 02:45

cwfitzgerald self-assigned this Jun 25, 2025

SupaMaggie70Incorporated closed this Jun 25, 2025

SupaMaggie70Incorporated reopened this Jun 25, 2025

Merge branch 'trunk' into precompiled-shaders

0f17512

SupaMaggie70Incorporated and others added 5 commits June 27, 2025 14:29

Merge branch 'trunk' into precompiled-shaders

34d076a

How do these things even happen?

6d49154

Merge branch 'trunk' into precompiled-shaders

4b19ca3

Merge branch 'trunk' into precompiled-shaders

e7cc939

More work, adapted with some of Cw's advice

18dffd5

Merge branch 'trunk' into precompiled-shaders

ca381ca

cwfitzgerald requested changes Jul 10, 2025

View reviewed changes

SupaMaggie70Incorporated and others added 3 commits July 10, 2025 17:20

Merge branch 'trunk' into precompiled-shaders

681857e

Various changes as requested from review

b542ba5

Fixed issues with trace

ff61dde

SupaMaggie70Incorporated requested a review from cwfitzgerald July 11, 2025 02:28

SupaMaggie70Incorporated added 2 commits July 11, 2025 13:46

Merge branch 'trunk' into precompiled-shaders

1e454b2

Merge branch 'trunk' into precompiled-shaders

3ae8b38

SupaMaggie70Incorporated and others added 4 commits July 16, 2025 10:18

Merge branch 'trunk' into precompiled-shaders

7f4e64f

Merge branch 'trunk' into precompiled-shaders

0b02fda

Fixed accidental merge change

8b48cc5

Merge branch 'trunk' into precompiled-shaders

9c84ef0

jimblandy assigned DJMcNab Aug 6, 2025

DJMcNab reviewed Aug 6, 2025

View reviewed changes

DJMcNab removed their assignment Aug 6, 2025

SupaMaggie70Incorporated added 7 commits August 8, 2025 22:58

Merge branch 'trunk' into precompiled-shaders

f864fcd

Tried to fix the tests and whatnot

cfb676f

More small tweaks

ab1e89e

A few more tweaks

7dde16f

Tried to fix spirv issue (I'm blind to vulkan compile rn)

8a9cbbd

Ok thats embarassing

d568b12

Final pass (single word changed in doc comment)

b584304

SupaMaggie70Incorporated requested a review from DJMcNab August 9, 2025 05:29

DJMcNab removed their request for review August 11, 2025 08:05

jonahnm mentioned this pull request Aug 12, 2025

Unable to add HLSL_DXIL_SHADER_PASSTHROUGH to DX12 device features. #8089

Open

		/// Optional reflection information
		pub reflection: Option<ShaderModuleReflection>,

Initial precompiled shaders implementation #7834

Are you sure you want to change the base?

Initial precompiled shaders implementation #7834

Uh oh!

Conversation

SupaMaggie70Incorporated commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwfitzgerald commented Jun 25, 2025

Uh oh!

SupaMaggie70Incorporated commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwfitzgerald commented Jun 25, 2025

Uh oh!

SupaMaggie70Incorporated commented Jun 25, 2025

Uh oh!

cwfitzgerald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cwfitzgerald Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SupaMaggie70Incorporated commented Jul 16, 2025

Uh oh!

jimblandy commented Aug 6, 2025

Uh oh!

DJMcNab left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SupaMaggie70Incorporated commented Aug 6, 2025

Uh oh!

SupaMaggie70Incorporated commented Aug 9, 2025

Uh oh!

DJMcNab commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

SupaMaggie70Incorporated commented Jun 20, 2025 •

edited

Loading

SupaMaggie70Incorporated commented Jun 25, 2025 •

edited

Loading

cwfitzgerald Jul 10, 2025 •

edited

Loading

DJMcNab commented Aug 11, 2025 •

edited

Loading